upgrade: pagination package upgrade for Solid 2.0#871
Conversation
🦋 Changeset detectedLatest commit: b3a91c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/pagination/src/index.ts`:
- Around line 398-403: The Promise returned by fetcher(currentPage) can reject
and leaves fetching stuck true; wrap the call so rejections are handled (use
.catch(...) or try/catch and a .finally) and always call setFetching(false) even
on error, while preserving the cancelled check and existing behavior for empty
content (use the same symbols: fetcher, currentPage, cancelled, setEnd,
setPages, setFetching); optionally log the error or setEnd(true) on fatal
failures to avoid deadlock and prevent unhandled rejections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fed58dc-f439-4b4c-a4ff-813e3104b0b7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.changeset/pagination-solid2-migration.mdpackages/pagination/README.mdpackages/pagination/package.jsonpackages/pagination/src/index.tspackages/pagination/test/index.test.tspackages/pagination/test/server.test.ts
| fetcher(currentPage).then(content => { | ||
| if (cancelled) return; | ||
| if (content.length === 0) setEnd(true); | ||
| setPages(p => [...p, ...content]); | ||
| setFetching(false); | ||
| }); |
There was a problem hiding this comment.
Handle rejected fetches so infinite scroll doesn’t deadlock.
If fetcher(currentPage) rejects, setFetching(false) is never reached and fetching() stays true, which blocks further observer-driven loads and can emit unhandled rejections.
Suggested fix
- fetcher(currentPage).then(content => {
- if (cancelled) return;
- if (content.length === 0) setEnd(true);
- setPages(p => [...p, ...content]);
- setFetching(false);
- });
+ fetcher(currentPage)
+ .then(content => {
+ if (cancelled) return;
+ if (content.length === 0) setEnd(true);
+ setPages(p => [...p, ...content]);
+ })
+ .catch(() => {
+ if (cancelled) return;
+ // optionally expose/log error state if desired
+ })
+ .finally(() => {
+ if (!cancelled) setFetching(false);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fetcher(currentPage).then(content => { | |
| if (cancelled) return; | |
| if (content.length === 0) setEnd(true); | |
| setPages(p => [...p, ...content]); | |
| setFetching(false); | |
| }); | |
| fetcher(currentPage) | |
| .then(content => { | |
| if (cancelled) return; | |
| if (content.length === 0) setEnd(true); | |
| setPages(p => [...p, ...content]); | |
| }) | |
| .catch(() => { | |
| if (cancelled) return; | |
| // optionally expose/log error state if desired | |
| }) | |
| .finally(() => { | |
| if (!cancelled) setFetching(false); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/pagination/src/index.ts` around lines 398 - 403, The Promise
returned by fetcher(currentPage) can reject and leaves fetching stuck true; wrap
the call so rejections are handled (use .catch(...) or try/catch and a .finally)
and always call setFetching(false) even on error, while preserving the cancelled
check and existing behavior for empty content (use the same symbols: fetcher,
currentPage, cancelled, setEnd, setPages, setFetching); optionally log the error
or setEnd(true) on fatal failures to avoid deadlock and prevent unhandled
rejections.
1 participant
Migrate
@solid-primitives/paginationto Solid.js v2.0 beta.10. ReplacescreateResource(removed) with a manual fetch + cancellation effect, replacescreateComputed(removed) with a derived memo for page clamping, movesisServerto@solidjs/web, and removesbatch().Summary by CodeRabbit
Release Notes
Breaking Changes
@solidjs/webpeer dependencycreateInfiniteScrollno longer exposespages.loadingandpages.errorpropertiesisServermust now be imported from@solidjs/webcreateInfiniteScrollreturn type:loaderis now a ref function instead of a directiveDocumentation